Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plans: First pass at adding Jetpack auto-config thank you page #11692

Merged
merged 1 commit into from
Mar 3, 2017

Conversation

ebinnion
Copy link
Contributor

@ebinnion ebinnion commented Mar 1, 2017

This PR is meant to add the auto-config functionality below the PlanThankYouCard block. This PR is functional at this point, but needs a lot more polish before this feature can be finished. I'd like to get the PR in now since it's already getting large and it'll help with review.

Much of the functionality is copied from the jetpack-plugins-setup.jsx component. That is not ideal. In the future, once I polish everything and finish the functionality, I'd like to maybe create a data component that handles fetching and deciding when to move from plugin to plugin. Then, I can pass that as props down to the jetpack-plugins-setup.jsx component and this new component.

But, I'd rather finish this before I start making decisions about the data component.

This functionality is behind a feature flag, so there shouldn't be any issue with merging.

screen shot 2017-03-02 at 4 46 02 pm

To test:

You'll want to go to /checkout/thank-you/$site/$receipt where $site is a Jetpack site ID and $receipt is a recent purchase for a Jetpack plan.

You'll need to have Jetpack connected and active, and then this should install/activate VaultPress and/or Akismet. You can verify this in wp-admin afterwards.

cc @lezama @ryelle for review and thoughts on the approach.

@ebinnion ebinnion added .org Plans Jetpack [Feature] Plans & Upgrades All of the plans on WordPress.com and flow for upgrading plans. [Status] In Progress labels Mar 1, 2017
@ebinnion ebinnion self-assigned this Mar 1, 2017
@matticbot
Copy link
Contributor

@matticbot matticbot added the [Size] M Medium sized issue label Mar 1, 2017
@ebinnion ebinnion force-pushed the add/first-pass-jetpack-plan-update branch from f830822 to 6f238f4 Compare March 2, 2017 20:15
@matticbot matticbot added [Size] XL Probably needs to be broken down into multiple smaller issues and removed [Size] M Medium sized issue labels Mar 2, 2017
}
) }
<NoticeAction href={ manageUrl }>
{ translate( 'Turn On Manage' ) }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! I've found a possible matching string that has already been translated 22 times:
translate( 'Turn on remote management' ) ES Score: 9

Help me improve these suggestions: react with 👎 if the suggestion doesn't make any sense, or with 👍 if it's a particularly good one (even if not implemented).

} else if ( 'akismet' === plugin.slug ) {
switch ( plugin.status ) {
case 'done':
return translate( 'Spam protection active' );
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! I've found a possible matching string that has already been translated 27 times:
translate( 'Spam Protection' ) ES Score: 9
See 2 additional suggestions in the PR translation status page

Help me improve these suggestions: react with 👎 if the suggestion doesn't make any sense, or with 👍 if it's a particularly good one (even if not implemented).

) }
>
<NoticeAction href={ manageUrl }>
{ translate( 'Turn On Manage' ) }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! I've found a possible matching string that has already been translated 22 times:
translate( 'Turn on remote management' ) ES Score: 9

Help me improve these suggestions: react with 👎 if the suggestion doesn't make any sense, or with 👍 if it's a particularly good one (even if not implemented).

@ebinnion ebinnion force-pushed the add/first-pass-jetpack-plan-update branch from 85fcc1a to 7ecf79d Compare March 2, 2017 22:39
@matticbot matticbot added the [Size] XL Probably needs to be broken down into multiple smaller issues label Mar 2, 2017
@ebinnion ebinnion added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Mar 2, 2017
@ebinnion ebinnion requested review from lezama and ryelle March 2, 2017 22:52
import { fetchPluginData } from 'state/plugins/wporg/actions';
import { requestSites } from 'state/sites/actions';
import {
installPlugin,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could go inline with import :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)

}

allPluginsHaveWporgData() {
const plugins = this.addWporgDataToPlugins( this.props.plugins );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we shouldn't call addWporgDataToPlugins from allPluginsHaveWporgData

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may not need the wporg plugins data any longer since we're currently just using the plugin slug. I believe that's a holdover from the old UI where we wanted to show the plugin icon and the plugin name. I'll look into that in future iterations.

@lezama
Copy link
Contributor

lezama commented Mar 3, 2017

looks good for a first iteration

@lezama lezama closed this Mar 3, 2017
@lezama lezama reopened this Mar 3, 2017
@lezama lezama added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 3, 2017
@ebinnion ebinnion merged commit edbd5f7 into master Mar 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Plans & Upgrades All of the plans on WordPress.com and flow for upgrading plans. Jetpack [Size] XL Probably needs to be broken down into multiple smaller issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants